[One Workflow] Fix false validation errors for template-local variables in Liquid templates by dej611 · Pull Request #253405 · elastic/kibana

dej611 · 2026-02-17T10:52:26Z

Summary

This PR fixes validation and autocomplete for variables defined locally within Liquid templates ({% assign %}, {% capture %}, {% for %}). Previously, the YAML editor would report these as "Variable X is not valid in this context" even though they are perfectly valid at runtime. The editor now parses the Liquid AST to discover template-local variables, infers their types where possible, and extends the workflow context schema accordingly. Both the validation and autocomplete paths benefit from these changes.

Problem

When writing workflow YAML like:

steps:
  - name: greet
    type: console
    with:
      message: |
        {% assign greeting = "Hello" %}
        {{ greeting }}

The editor would flag greeting as an invalid variable because the Zod context schema only knew about workflow-level context (e.g. event, steps, workflow) and had no awareness of variables introduced by Liquid tags within the same template string.

What changed

⚡ Uses a LiquidJS parser (with LRU cache to avoid memory overuse) to build the AST for type inference
🏷️ Extracts from the AST variables and types
🔧 Maps type to Zod schema types
🐛 Improve validation with a simplified flow
⚡ Improve scalar extract to be faster with binary search
🐛 Autocomplete is now fed with in scope variables

Type inference details

The inferSchemaFromAssignRhs function resolves types as follows:

RHS expression	Inferred type
`42`, `-3.14`	`z.number()`
`"hello"`, `'world'`	`z.string()`
`true`, `false`	`z.boolean()`
`steps.x.outputs.value`	Schema resolved from path
Anything else	`z.unknown()`

For for-loops, the item type is resolved by looking up the collection path's schema and extracting its array element type via getForeachItemSchema.

Known limitations

Liquid filters are not type-aware. Filters are stripped before inference, so {% assign count = items | size %} will infer the type of items (array) rather than number. This is documented in code comments.
Conditional branches are not distinguished. An assign inside {% if %}...{% else %} is treated as unconditionally available once its tag position is before the cursor. The variable's type is based on whichever assign is last in source order, not a union of branches.
{% increment %}/{% decrement %} are not extracted. Variables introduced by these tags will still be flagged as unknown.
Two parallel extractors exist. The validation path (validate_workflow_yaml/lib/extract_template_local_context.ts) and the context/autocomplete path (workflow_context/lib/extract_template_local_context.ts) have separate implementations. The context version is richer (tracks RHS for type inference), while the validation version only collects names. This duplication exists because the two call sites have different data needs and were authored incrementally.
Block scalar offset is approximate. For YAML block literals (|) and folded scalars (>), the mapping from document offset to template-string offset is a best-effort estimate (offset - scalarStart, clamped to template length). This slightly overestimates the position but is safe -- it means variables defined before the cursor are always included.
For-loop item type falls back to z.unknown() when the collection path cannot be resolved against the schema (e.g., the collection is a template-local variable itself).

Testing examples with workflows

Detailed testing examples

Example 1: Assign with literal

steps:
  - name: greet
    type: console
    with:
      message: '{% assign greeting = "Hello World" %}The greeting is: {{ greeting }}'

Before: greeting flagged as "Variable greeting is not valid in this context."
After: greeting recognized as string, no error.

Example 2: Assign with path reference

inputs:
  - name: message
     type: string
     default: "Hello world"

steps:
  - name: console
    type: console
    with:
      message: |
        ## Assign inherits basic types from inputs/consts
        {% assign bar = inputs.message %}
        {{bar}}

Before: bar flagged as unknown.
After: bar schema resolved from inputs.message.

Example 3: Capture block

steps:
  - name: build_url
    type: console
    with:
      message: '{% capture base_url %}https://example.com/api{% endcapture %}{{ base_url }}/data'

Before: base_url flagged as unknown.
After: base_url recognized as string.

Example workflow:

name: New workflow
enabled: false
description: This is a new workflow
triggers:
  - type: manual

consts:
  items:
    - type: item
      children:
        - name: one
  analyst:
    type: object
    properties:
      email: analyst@example.com

inputs:
  properties:
    analyst:
      type: object
      properties:
        email:
          type: string
          format: email
          default: "analyst@example.com"

steps:
  - name: hello_test
    type: console
    with:
      message: |
        ## Capture works
        {%- capture person -%}{ "name" : "John Doe" , "age" : 20 }{%- endcapture -%}{{person}}
        ## Assign infer correctly basic types
        {% assign foo = false %}{{foo}}
        ## Variable rewrite
        {% assign x = 42 %}{% assign x = "now a string" %}{{ x }}
        ## Array are partially handled
        {% assign count = consts.items %}{{ count }}
        ## Object support
        {{consts.analyst.properties.email}}

Checklist

Unit or functional tests were updated or added to match the most common scenarios

Kiryous · 2026-02-17T14:14:15Z

+  }
+}
+
+function extractFromTemplates(


should we use it on the on the variables collection level?

https://github.com/elastic/kibana/blob/main/src/platform/plugins/shared/workflows_management/public/features/validate_workflow_yaml/lib/collect_all_variables.ts

I see the code in that area uses Regular Expressions in order to parse the yaml, while here I've preferred the generation of the LiquidJS AST and the visit pattern directly.
I'm not sure how much compatibility there is between the two parts and do not want to change the existing regular expressions to be more complex than the current state.
I can see as a follow up a rewrite of the logic to use the LiquidJS AST but I would say it's out of scope for this specific issue.

rosomri

Nicely done 👑 The implementation is pretty nice and clearly fixes the problem, the test coverage is solid too.

I wanted to raise one concern and share an idea I explored (with Cursor's help, haven't tested end-to-end locally - so take it as a direction to validate).

Alternative approach: leveraging LiquidJS built-in static analysis

The concern

The PR introduces custom AST walking over LiquidJS internals — the EnrichedTemplate type, processTag, manual scope tracking for assign/capture/for — to discover template-local variables. This reimplements scoping rules that LiquidJS already knows, and relies on undocumented internal properties (tag.name, tag.variable, tag.templates, tag.token.args). I worry this is fragile: a LiquidJS minor version bump could silently break these assumptions.

LiquidJS has a built-in `analyzeSync` API

Turns out LiquidJS already exposes a static analysis API that gives us exactly what the custom code builds manually:

const { Liquid } = require('liquidjs');
const engine = new Liquid();

const template = '{% assign greeting = "Hello" %}{{ greeting }} {{ workflow.name }} {{ unknown }}';
const analysis = engine.parseAndAnalyzeSync(template);

analysis returns:

interface StaticAnalysis {
  variables: Variables;  // ALL variable usages with row/col locations
  globals: Variables;    // Only context/external variables (not locally defined)
  locals: Variables;     // Only template-local variables (assign, capture, increment)
}

Each variable includes full path segments and { row, col } location. Quick comparison:

Feature	`analyzeSync`	Custom AST walking (this PR)
`assign` locals	✅	✅
`capture` locals	✅	✅
`for` loop iterators / `forloop`	✅ (excluded from `globals`)	✅
`increment` / `decrement`	✅	❌ (acknowledged limitation)
Nested scopes	✅	✅
Position info	✅ (`row`, `col`)	✅ (custom offset math)

How it could plug into the existing validation

The key insight: if we replace the regex variable collector (collectAllVariables) with analyzeSync and only feed globals into the existing Zod validation pipeline, template-local variables never enter validation in the first place — the false-positive problem disappears without needing to extend the schema at all.

// Instead of regex-collecting ALL {{ }} references...
// Use analyzeSync per scalar value to get only globals:
const analysis = engine.parseAndAnalyzeSync(scalarValue);

// Only validate globals against the Zod context schema (existing pipeline)
for (const [name, vars] of Object.entries(analysis.globals)) {
  for (const variable of vars) {
    const path = variable.toString();        // e.g. "workflow.name"
    const { row, col } = variable.location;  // position for squiggly

    // Feed into existing getSchemaAtPath / validateVariable — unchanged
    const { schema } = getSchemaAtPath(contextSchema, path);
    // ... existing validation logic
  }
}

// Locals are available for autocomplete without custom AST walking:
// analysis.locals → use for suggestions

This would keep getContextSchemaForPath, validateVariable, and getSchemaAtPath exactly as they are today, while eliminating the need for extract_template_local_context.ts, extend_context_with_template_locals.ts, the EnrichedTemplate type, the custom processTag walker, and the liquid_parse_cache.ts singleton.

Bonus: `catchAllErrors` + `strictVariables`

LiquidJS also has a catchAllErrors option that collects all render errors in one pass instead of stopping at the first. Combined with strictVariables: true and a dummy context built from the Zod schema, renderSync would flag all truly undefined variables (both invalid globals and improperly scoped locals) natively. This is more involved since it requires building a stub context from the schema, but worth knowing about.

TL;DR

The current PR correctly solves the problem and I don't want to block it. But I think we'd end up with something simpler and more maintainable if we lean on LiquidJS's own static analysis (analyzeSync / globalFullVariablesSync) instead of reimplementing scope tracking. Less code, more resilient to library upgrades, and covers edge cases like increment/decrement for free.

Happy to pair on exploring this if it sounds worth pursuing — as I said, I poked at this through Cursor so it needs proper local validation before we commit to the direction.

rosomri · 2026-02-18T13:07:17Z

+  return maxEnd;
+}
+
+function visitChildren(tag: EnrichedTemplate, visit: (tpl: EnrichedTemplate) => void): void {


Note - the EnrichedTemplate type manually extends LiquidJS internal types (Template, Token) with undocumented properties (name, variable, templates, elseTemplates, branches, token.args). This is fragile: LiquidJS doesn't guarantee these internal shapes, and they could change in a minor version bump.

rosomri · 2026-02-18T13:08:02Z

- * This finds the scalar VALUE node (not the key) at the given position
+ * Cache of scalar value nodes per Document. Uses WeakMap so entries are
+ * garbage-collected when the Document is no longer referenced.
+ * Nodes are sorted by range start for O(log n) offset lookups.


dej611 · 2026-02-19T09:43:50Z

The current PR correctly solves the problem and I don't want to block it. But I think we'd end up with something simpler and more maintainable if we lean on LiquidJS's own static analysis (analyzeSync / globalFullVariablesSync) instead of reimplementing scope tracking. Less code, more resilient to library upgrades, and covers edge cases like increment/decrement for free.

Happy to pair on exploring this if it sounds worth pursuing — as I said, I poked at this through Cursor so it needs proper local validation before we commit to the direction.

Went into an exploring phase for this suggestion but unfortunately could not leverage the analyzeSync api: while it could correctly resolve the variable names, it will discard the right hand side of the expression, making is useless for type inference.
So I had to revert to the AST traversing approach, but this time went thru a full rewrite using only public types from the liquidjs library and safe typeguards.

kibanamachine · 2026-04-23T09:48:52Z